[FEAT] Add minSecondsBeforeRefresh + debounce refresh token#43
Open
itailevi98 wants to merge 6 commits intomainfrom
Open
[FEAT] Add minSecondsBeforeRefresh + debounce refresh token#43itailevi98 wants to merge 6 commits intomainfrom
minSecondsBeforeRefresh + debounce refresh token#43itailevi98 wants to merge 6 commits intomainfrom
Conversation
src/server/app-router.ts
Outdated
| return undefined | ||
| } | ||
|
|
||
| export async function getUserAndAccessToken(): Promise<{ user?: UserFromToken; accessToken?: string }> { |
Contributor
There was a problem hiding this comment.
Could you change this type so they are either both set or both unset?
src/server/app-router.ts
Outdated
| accessToken: string | ||
| } | ||
| | { | ||
| user: never |
Contributor
There was a problem hiding this comment.
this should be undefined
src/client/AuthProvider.tsx
Outdated
| // If we were offline or on a different tab, when we return, refetch auth info | ||
| // Some browsers trigger focus more often than we'd like, so we'll debounce a little here as well | ||
| const refreshOnOnlineOrFocus = async function () { | ||
| if (lastRefresh && currentTimeSecs() > lastRefresh + minSecondsBeforeRefresh) { |
Contributor
There was a problem hiding this comment.
We should still refresh in the case where lastRefresh is undefined to be safe
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Done as part of: https://www.notion.so/Client-Library-Updates-11753e303ce68051abf0cf4d570ee98e
Background
The original behavior before these changes was that in an application, on page focus, the
/userinfoendpoint would be called each time. This was unnecessary, and would spam these network calls. Now, a user can pass in a propminSecondsBeforeRefreshthat will only call the endpoint after that time has passed. This logic is the same as in the JS library.As well as this, a new function was added to get the user and access token form the server.
Smoke Tests
/userinfoendpoint only gets called after those seconds are up, and after clicking on the page to focus it.getuserAndAccessTokenfunction returned botht he user and access token as expected.